Skip to content

Disallow negative bare values#14453

Merged
philipp-spiess merged 6 commits into
nextfrom
fix/negative-bare-values
Sep 18, 2024
Merged

Disallow negative bare values#14453
philipp-spiess merged 6 commits into
nextfrom
fix/negative-bare-values

Conversation

@philipp-spiess

@philipp-spiess philipp-spiess commented Sep 18, 2024

Copy link
Copy Markdown
Contributor

Right now, it is possible to type grid-cols--8 which maps to:

/* Specificity: (0, 1, 0) */
.grid-cols--8 {
  grid-template-columns: repeat(-8, minmax(0, 1fr));
}

This doesn't make sense so we used this opportunity to audit all variants and utilities and properly disallow negative bare values. Utilities where negative values are supported still work by using the negative utility syntax, e.g.: -inset-4.

@philipp-spiess philipp-spiess requested review from RobinMalfait, adamwathan and thecrypticace and removed request for RobinMalfait September 18, 2024 12:07
RobinMalfait
RobinMalfait approved these changes Sep 18, 2024
View reviewed changes

@RobinMalfait RobinMalfait left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice I like it. There are still situations where a negative value makes sense e.g.: -z-10, but then -z-10 should be used instead of z--10.

For utilities with fractions, even if negatives make sense then you can always fallback to an arbitrary value.

@philipp-spiess

philipp-spiess commented Sep 18, 2024

Copy link
Copy Markdown
Contributor Author

@RobinMalfait Yep, agree. Double - feels super odd so better not even start to support it.

adamwathan
adamwathan reviewed Sep 18, 2024
View reviewed changes
Comment thread packages/tailwindcss/src/utilities.ts Outdated
@philipp-spiess philipp-spiess force-pushed the fix/negative-bare-values branch from 593f58a to 0102c81 Compare September 18, 2024 14:21
@philipp-spiess philipp-spiess merged commit 6ca8cc6 into next Sep 18, 2024
@philipp-spiess philipp-spiess deleted the fix/negative-bare-values branch September 18, 2024 14:49
@karlhorky

karlhorky commented Sep 26, 2024 •
edited
Loading

Copy link
Copy Markdown

haha hi again @philipp-spiess 👋

I just upgraded from 4.0.0-alpha.24 -> 4.0.0-alpha.25 and I was wondering whether this change led to this new failure in alpha.25:

./app/global.css
Error: Cannot apply unknown utility class: from-26.56%
    at onInvalidCandidate (/home/runner/work/upleveled.io/upleveled.io/node_modules/.pnpm/tailwindcss@4.0.0-alpha.25/node_modules/tailwindcss/dist/lib.js:8:108664)
    at L (/home/runner/work/upleveled.io/upleveled.io/node_modules/.pnpm/tailwindcss@4.0.0-alpha.25/node_modules/tailwindcss/dist/lib.js:8:106318)
    at /home/runner/work/upleveled.io/upleveled.io/node_modules/.pnpm/tailwindcss@4.0.0-alpha.25/node_modules/tailwindcss/dist/lib.js:8:108628

This is for a gradient stop percentage value.

Is this also another class that is not allowed any longer? (also: maybe this change was not related to this PR but another one instead...? 🤔)

@philipp-spiess

philipp-spiess commented Sep 27, 2024

Copy link
Copy Markdown
Contributor Author

@karlhorky Hey! Yeah we unfortunately had a bunch of places where we supported decimal places unknowingly. These transition breakpoints were one of those. This was never intended to work but sneaked in because we had an inconsistency in the way we interpret these values.

For anything that is not a whole integer, the idea is that you use arbitrary values. So in your case you can use from-[26.56%].

@karlhorky

karlhorky commented Sep 27, 2024

Copy link
Copy Markdown

Ok thanks!

I'll switch to arbitrary values for now, and in future, consider using integer percentages.

@coderabbitai coderabbitai Bot mentioned this pull request Jun 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adamwathan adamwathan adamwathan left review comments

@RobinMalfait RobinMalfait RobinMalfait approved these changes

@thecrypticace thecrypticace Awaiting requested review from thecrypticace

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@philipp-spiess @karlhorky @RobinMalfait @adamwathan